Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TreeSelect: Deprecate 36px default size #67855

Merged
merged 5 commits into from
Dec 13, 2024
Merged

Conversation

mirka
Copy link
Member

@mirka mirka commented Dec 11, 2024

Part of #65751

What?

Deprecate the 36px default size on TreeSelect.

Testing Instructions

  • Unit tests pass
  • Storybook stories should not log console warnings
  • All code snippets in documentation (JSDoc, Storybook, README) should have the __next40pxDefaultSize prop enabled

@mirka mirka added [Type] Code Quality Issues or PRs that relate to code quality [Package] Components /packages/components Needs Dev Note Requires a developer note for a major WordPress release cycle labels Dec 11, 2024
@mirka mirka self-assigned this Dec 11, 2024
@@ -136,7 +136,7 @@ export interface InputBaseProps extends BaseProps, FlexProps {
* If you want to apply standard padding in accordance with the size variant, wrap the element in
* the provided `<InputControlPrefixWrapper>` component.
*
* @example
* ```jsx
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tweaking so the code snippets render correctly in a Markdown file.

@@ -1,10 +1,10 @@
# TreeSelect

TreeSelect component is used to generate select input fields.
<!-- This file is generated automatically and cannot be edited directly. Make edits via TypeScript types and TSDocs. -->
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Taking this opportunity to convert this to an auto-generated README.

Comment on lines +27 to +30
/**
* A function that receives the value of the new option that is being selected as input.
*/
onChange?: SelectControlSingleSelectionProps[ 'onChange' ];
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overriding the prop description here because the original description includes mentions of a multiple prop, which doesn't exist in this component.

@mirka mirka marked this pull request as ready for review December 12, 2024 19:36
@mirka mirka requested a review from ajitbohra as a code owner December 12, 2024 19:36
@mirka mirka requested a review from a team December 12, 2024 19:36
Copy link

github-actions bot commented Dec 12, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: mirka <[email protected]>
Co-authored-by: tyxla <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copy link

Flaky tests detected in dddaa1f.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/12303474413
📝 Reported issues:

Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a minor docs omission nit, but other than that, LGTM 👍

Feel free to 🚢 after addressing! 🚀

## Props

The set of props accepted by the component will be specified below.
Props not included in this set will be applied to the SelectControl component being used.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is useful information that we removed by auto-generating the README. Can we make sure it gets included?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmm right, I think this is something we need to address at the package level. The majority of our components that do pass down props, we don't say that explicitly (e.g. in the canonical Storybook props docs), even though the TS types include them.

Extracted to #67979

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to be addressed as a follow-up 👍

@mirka mirka merged commit 3d17c61 into trunk Dec 13, 2024
71 of 72 checks passed
@mirka mirka deleted the tree-select-deprecate-size branch December 13, 2024 14:05
@github-actions github-actions bot added this to the Gutenberg 20.0 milestone Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Dev Note Requires a developer note for a major WordPress release cycle [Package] Components /packages/components [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants